Skip to content

feat: add fiber type definitions and update core types#33

Open
MarcelOlsen wants to merge 5 commits into
fiber/cleanupfrom
fiber/types
Open

feat: add fiber type definitions and update core types#33
MarcelOlsen wants to merge 5 commits into
fiber/cleanupfrom
fiber/types

Conversation

@MarcelOlsen

@MarcelOlsen MarcelOlsen commented Feb 13, 2026

Copy link
Copy Markdown
Owner

Add Fiber, FiberRoot, Hook, Effect types with WorkTag, Lane, and Flags
branded types. Update core/types.ts to export hook-related types and
remove old VDOMInstance.

Summary by CodeRabbit

  • New Features
    • Expanded public type exports including hook-related types and core state management types.
    • Introduced comprehensive fiber type system with branded numeric types for priority lanes and effect flags.
    • Added type guard utilities for improved runtime type checking and type safety.

@coderabbitai

coderabbitai Bot commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Fix all issues with AI agents
In `@src/core/types.ts`:
- Around line 114-125: Remove the duplicated type definitions (EffectCallback,
DependencyList, Reducer, MutableRefObject) from core/types and instead re-export
them from the canonical source where they are defined; locate the duplicate
declarations for EffectCallback, DependencyList, Reducer<State, Action>, and
MutableRefObject<T> in core/types and replace them with export-only statements
that re-export the same symbols from the hooks types module so there is a single
source of truth.
- Around line 94-95: The import "import type { PortalElement } from
\"../portals/types\";" is placed after runtime constants; move this type-only
import up into the existing import block at the top of the file alongside the
other imports (around the current imports near line 5) and remove the
duplicate/old import at its current location so linters and style rules are
satisfied.

In `@src/fiber/types.ts`:
- Around line 504-512: The isHostFiber type guard is incorrect because it
includes WorkTag.HostRoot but narrows stateNode to Element | Text
(HostRoot.stateNode is a FiberRoot), so either remove HostRoot from
isHostFiber's truthy tags or split into two guards: keep isHostFiber for
HostRoot if you want to detect root fibers (with its proper FiberRoot stateNode)
and add a new isDOMFiber (or similar) that returns fiber is Fiber & { stateNode:
Element | Text } and only checks WorkTag.HostComponent and WorkTag.HostText;
update callsites to use the appropriate guard (isDOMFiber when you need DOM
operations and isHostFiber when you need root detection).
- Around line 109-134: Replace the repeated "(... as number)" casts by adding a
small helper that builds a combined Flags value and delegates to createFlags;
implement a combineFlags(...flags: Flags): Flags that reduces flags with bitwise
OR (casting each flag to number inside the reducer) and passes the result into
createFlags, then update uses like PlacementAndUpdate, Deletion, MutationMask,
LayoutMask, PassiveMask to call combineFlags(Placement, Update, ...) instead of
calling createFlags with many explicit casts, keeping createFlags and the
original flag names (Placement, Update, ChildDeletion, ContentReset, Ref,
Hydrating, Visibility, Callback, Passive) as the references.
- Around line 254-261: The JSDoc for the stateNode field mentions "For class
components: the instance" but the stateNode union type on the Fiber interface
does not include a class instance type; update the declaration to prevent this
becoming a stale comment by either adding a clear TODO comment referencing
ClassComponent/WorkTag and stateNode (e.g., "TODO: add class component instance
type when implementing ClassComponent") or add a placeholder union member for
the class instance type (a named placeholder type) so the comment and the
stateNode type stay consistent; make the change near the stateNode declaration
and reference ClassComponent and WorkTag in the note.
- Around line 165-171: The Effect.type currently uses a plain number for the tag
field which loses type safety; change the tag property to use the const-backed
HookEffectTag union instead (e.g. define a type alias like `type
HookEffectTagType = typeof HookEffectTag[keyof typeof HookEffectTag]` and update
`export type Effect = { tag: HookEffectTagType; ... }`) so only valid
HookEffectTag values can be assigned; update the tag declaration in the Effect
type and add the small alias near the HookEffectTag definition if not already
present.
- Line 94: The file defines both a value constant named Update (export const
Update = createFlags(...)) and a generic type Update<S>, which is confusing;
rename the flag constant (e.g., to UpdateFlag or UpdateEffect) and update all
uses of that constant (including any combined flag masks and exports) to the new
name; ensure the createFlags(...) call and its export remain intact and update
any imports/refs elsewhere to the new symbol while leaving the generic type
Update<S> unchanged.
- Around line 487-495: The forward-declared MiniReactContext in fiber/types.ts
does not match the real context shape in context/types.ts (real has _contextId,
Provider: FunctionalComponent<...>, no Consumer, T = unknown), so replace the
lie with a single canonical type: import the actual context type from
context/types.ts (use a type-only import like "import type { Context } from
'context/types'") or move the canonical interface into a shared module and have
both files import it; ensure the symbol names match (update MiniReactContext to
the real interface shape including _contextId and Provider's FunctionalComponent
signature and remove Consumer) so the types are identical and the circular
dependency is handled via type-only imports or a new shared type file.
📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef69e5 and 5d59de0.

📒 Files selected for processing (2)
  • src/core/types.ts
  • src/fiber/types.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: MarcelOlsen
Repo: MarcelOlsen/mini-react PR: 8
File: README.md:701-708
Timestamp: 2025-06-17T12:40:39.737Z
Learning: MarcelOlsen prefers to keep test documentation simple with just one example of running specific test files, rather than listing detailed commands for each test suite in the README.
🧬 Code graph analysis (2)
src/fiber/types.ts (1)
src/core/types.ts (2)
  • ElementType (27-32)
  • AnyMiniReactElement (7-15)
src/core/types.ts (1)
src/hooks/types.ts (4)
  • EffectCallback (73-73)
  • DependencyList (74-74)
  • Reducer (82-82)
  • MutableRefObject (109-111)
🔇 Additional comments (5)
src/fiber/types.ts (5)

15-36: OH LET'S GO! This is the good stuff right here!

const object + satisfies pattern instead of enums? That's the way. Enums in TypeScript are... look, they're weird, they generate runtime code, they don't tree-shake well. This pattern gives you the type safety AND the const values infer correctly. Chef's kiss.


42-66: Branded types! Chat, this is actual type safety!

You can't accidentally pass a Lane where you need Flags anymore. The unique symbol brand pattern is THE way to do nominal typing in TypeScript. No more "oops I passed a priority number where I needed a flag bitmask" bugs. This is defensive programming done RIGHT.


173-182: Hook linked list structure - looks good!

Using unknown for the state types is the right call here since hooks can hold literally anything. The linked list pattern matches React internals. Solid.


395-448: FiberRoot type is STACKED with all the scheduling goodness!

Lanes for pending, suspended, pinged, expired work - this is the concurrent mode infrastructure. Callbacks, timing, hydration state - all there. The comments explain what each field does. This is well-architected for a mini-React implementation. Let's go!


454-481: Type utilities with conditional types - this is TypeScript mastery!

FiberOfTag<T>, FiberPropsFor<T>, StateNodeFor<T> - these narrowing utilities are gonna save so much casting in the reconciler code. Conditional type mapping based on WorkTag? Beautiful. This is the kind of type infrastructure that makes the runtime code actually type-safe instead of any-everything.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread src/core/types.ts Outdated
Comment thread src/core/types.ts Outdated
Comment thread src/fiber/types.ts Outdated
Comment thread src/fiber/types.ts Outdated
Comment thread src/fiber/types.ts
Comment thread src/fiber/types.ts
Comment thread src/fiber/types.ts Outdated
Comment thread src/fiber/types.ts Outdated
Add Fiber, FiberRoot, Hook, Effect types with WorkTag, Lane, and Flags
branded types. Update core/types.ts to export hook-related types and
remove old VDOMInstance.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@src/core/types.ts`:
- Around line 110-111: The import for StateOrEffectHook is located after runtime
constants; move the import type declaration for StateOrEffectHook to the top of
the file together with the other imports so all imports are grouped, and delete
the stray import statement that currently sits mid-file (the duplicate import of
StateOrEffectHook).

In `@src/fiber/types.ts`:
- Around line 532-536: The type guard isPortal currently only narrows fiber.tag
but not fiber.stateNode; update its return type to narrow stateNode to
PortalStateNode so callers can access portal-specific fields (e.g.,
stateNode.containerInfo) without casting. Specifically, change the predicate
signature from "fiber is Fiber & { tag: typeof WorkTag.HostPortal }" to include
"& { tag: typeof WorkTag.HostPortal; stateNode: PortalStateNode }" and ensure
any imports or references to PortalStateNode are used so TypeScript recognizes
the narrowed type in the isPortal function.
- Around line 146-147: Remove the redundant type alias HookEffectTagType and
replace all uses of HookEffectTagType with the original HookEffectTag type
(e.g., update the reference on the line that currently uses HookEffectTagType to
use HookEffectTag instead); specifically delete the declaration "export type
HookEffectTagType = HookEffectTag" and update any downstream references to the
alias (such as the usage on the line referenced in the review) to reference
HookEffectTag directly so the code uses the single canonical type name.
📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ab2a6c and 950542f.

📒 Files selected for processing (2)
  • src/core/types.ts
  • src/fiber/types.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: MarcelOlsen
Repo: MarcelOlsen/mini-react PR: 8
File: README.md:701-708
Timestamp: 2025-06-17T12:40:39.737Z
Learning: MarcelOlsen prefers to keep test documentation simple with just one example of running specific test files, rather than listing detailed commands for each test suite in the README.
🧬 Code graph analysis (1)
src/fiber/types.ts (1)
src/core/types.ts (2)
  • ElementType (28-33)
  • AnyMiniReactElement (8-16)
🔇 Additional comments (9)
src/core/types.ts (2)

5-6: LGTM! PortalElement import yeeted to the top where it belongs!

This fixes the previous import placement issue. Type imports at the top, runtime constants below. That's what I like to see, chat.


113-122: CLEAN. Re-export instead of duplicating. This is the way.

Chat, this is EXACTLY what I was talking about. Single source of truth. No more copy-paste type definitions drifting apart at 2 AM. You import from hooks, you re-export from core. Beautiful. DRY principle respected. Code that SCALES.

src/fiber/types.ts (7)

15-36: WorkTag with const object + satisfies? CHEF'S KISS, chat!

This is the pattern. No enums, no magic. You get your const object, you get your satisfies for type safety, you get your type extraction. Runtime values AND compile-time safety. This is TypeScript the way it was MEANT to be written.


42-68: Branded types with factory functions AND a combineFlags helper? We're cooking with GAS now!

Chat, this is CLEAN. Branded types prevent you from accidentally passing a regular number where a Lane should go. The factory functions make creation explicit. And that combineFlags helper? No more as number casts scattered everywhere. This is defensive TypeScript that scales.


93-128: UpdateEffect - perfect naming to avoid the collision!

Previous review flagged Update colliding with the Update<S> type. Now it's UpdateEffect. Different concepts, different names. No more confusion at 3 AM debugging session. And those combined flags using combineFlags? Clean. Readable. Maintainable.


160-166: Effect type now using the proper HookEffectTag type. No more number lies!

Previous review called out tag: number leaving type safety on the table. Now we got tag: HookEffectTagType (or just use HookEffectTag directly per my other comment). Either way, TypeScript will now YELL at you if you try to pass tag: 42. That's a bug prevented at compile time. This is why we TypeScript, chat.


249-257: TODO comment for ClassComponent - this is how you handle future work!

Previous review flagged the comment mentioning class instances without the type. Now you got a proper TODO with context. When someone implements ClassComponent, they'll see this. They'll know what to do. This is professional-grade code documentation.


480-484: Type-only import at the bottom to break circular deps - intentional and documented.

Look, normally I'd be SCREAMING about imports at the bottom. But this is INTENTIONAL. Comment explains it. Type-only import means no runtime impact. And by importing directly from context/types instead of forward-declaring, you avoid the type mismatch nightmare from the previous review. This is the right call.


494-509: isDOMFiber vs isHostFiber split - TYPE SAFETY ACHIEVED!

Chat, previous review caught a NASTY bug. Old isHostFiber included HostRoot but narrowed stateNode to Element | Text. That's a LIE. HostRoot.stateNode is FiberRoot, not a DOM node!

Now you got:

  • isDOMFiber: Narrows to Element | Text - use this for DOM operations
  • isHostFiber: Returns boolean - no narrowing, no lies

This is the FIX. Clean separation. No more runtime explosions when someone calls appendChild on a FiberRoot.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread src/core/types.ts Outdated
Comment thread src/fiber/types.ts Outdated
Comment thread src/fiber/types.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/fiber/types.ts`:
- Around line 449-476: StateNodeFor currently lacks a branch for HostPortal so
StateNodeFor<typeof WorkTag.HostPortal> resolves to null; update the conditional
type in StateNodeFor to include a branch mapping T extends typeof
WorkTag.HostPortal to PortalStateNode (ensure PortalStateNode type is
imported/available) so the HostPortal fibers get the correct stateNode type.
📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ab2a6c and f7af613.

📒 Files selected for processing (2)
  • src/core/types.ts
  • src/fiber/types.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: MarcelOlsen
Repo: MarcelOlsen/mini-react PR: 8
File: README.md:701-708
Timestamp: 2025-06-17T12:40:39.737Z
Learning: MarcelOlsen prefers to keep test documentation simple with just one example of running specific test files, rather than listing detailed commands for each test suite in the README.
🧬 Code graph analysis (1)
src/fiber/types.ts (1)
src/core/types.ts (2)
  • ElementType (29-34)
  • AnyMiniReactElement (9-17)
🔇 Additional comments (13)
src/core/types.ts (2)

5-7: Imports are grouped up top — clean.
No mid-file import whiplash; keeps TS lint happy.


111-120: Re-exporting hook types keeps DRY.
Single source of truth in hooks/types is the right move, chat.

src/fiber/types.ts (11)

11-36: WorkTag const object is tight.
as const satisfies gives the enum vibes without the runtime cost.


42-69: Branded lanes/flags + factories are solid.
combineFlags keeps the bitmask soup readable.


74-87: Lane constants are clear and explicit.
Binary values are easy to reason about when debugging scheduler stuff.


93-129: Flags + masks are consistent and clean.
Nice use of combineFlags for the masks.


134-200: Hook types are cohesive.
The Effect/Hook/Update models line up cleanly with the fiber layout.


206-212: PortalStateNode is minimal and correct.
No extra baggage, just the container — perfect.


225-334: Fiber shape is comprehensive and readable.
The TODO for ClassComponent is a good breadcrumb.


336-371: Supporting types are cohesive.
Refs + context deps + queues fit together without leaking abstractions.


376-443: FiberRoot metadata is well-scoped.
Scheduling + hydration fields are exactly what you'd expect.


482-484: Type-only context import avoids circular pain.
Clean separation, zero runtime impact.


493-536: Type guards are tight and useful.
isDOMFiber and isPortal give solid narrowing; no unsafe casts.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread src/fiber/types.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant